Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(contracts/misc): add V0 draft of Tag #2726

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

feat(contracts/misc): add V0 draft of Tag #2726

wants to merge 10 commits into from

Conversation

Zodomo
Copy link
Contributor

@Zodomo Zodomo commented Dec 20, 2024

Built out an initial draft of a crosschain tag game, playable via NFTs.

issue: none

@Zodomo Zodomo self-assigned this Dec 20, 2024
}

// Pay gas if requested
if (payGas) _sendGas(to);
Copy link
Contributor

@kevinhalliday kevinhalliday Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple things on _sendGas?

  • how do we make sure we have enough to service requests? prefunding, monitoring, and funding if needed?
  • if transfer in _sendGas fails, then the call reverts. NFT is not minted and user is not refunded. Is that what we want?

Sorry still reading, see you use gas pump for this. nice

@kevinhalliday
Copy link
Contributor

Sorry accidently submitted my review before finising. Still looking

Copy link
Contributor

@kevinhalliday kevinhalliday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice.

Let's start working on some tests. If requirements are still in flight and you want to wait that's fine. But if you don't expect big changes i say let's get testin. Lot of code here


function getTagCount(uint256 tokenId) public view returns (uint32) {
return _getTagCount(tokenId);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These ones that just proxy internal functions ^

Let's mark their public counterparts as external. Saves some gas. And otherwise there would be no reason for 2 separate functions

tagCooldown = 1 minutes;
price = 1 ether;

if (block.chainid != OMNI_CHAIN_ID) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to deploy on a non-mainnet network for testing, you can pass this in or just read it from the portal omniChainId() (from XApp) and set it to an immutable

function getPendingTagQueueLength(uint256 tokenId) public view returns (uint256) {
uint96 cursor = uint16(_getExtraData(tokenId) >> 80);
return tagQueue[tokenId].length - cursor;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment here (or elsewhere) explaining cursor plz

)
);
return requiredEth + pumpFee + feeFor(OMNI_CHAIN_ID, data, MINT_GAS_LIMIT);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So mints only mint on omni?

/* INTERNAL FUNCTIONS */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

function _hasClaimedWhitelist(address minter) public view returns (bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is still public

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants